Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Authenticated Sysinfo #16755

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

Authenticated Sysinfo #16755

wants to merge 2 commits into from

Conversation

w-e-w
Copy link
Collaborator

@w-e-w w-e-w commented Dec 27, 2024

Description

currently previously the the internal /internal/sysinfo /internal/sysinfo-download routes are not authenticated

I think AUTO's intention was that so the same API can be reused

but this has lead to a security issue in the past and could lead to more in the future if not already
so to be safe it's better that these routes require authenticated

this PR changes how sysinfo tabs downloads sysinfo, using javascript button clicks and gradio calls
(as opposed to browser to API calls)
and as an extra benefit there's now a progress effect on the HTML while retrieving sysinfo

the Sysinfo tab no longer no longer relies on the API
instead it's gradio button click event that generates sysinfo to a hidden text box
followed by JavaScript to download as file or open this info in new taba hidden textarea

this is overkill

normally the sysinfo should be in the realms of kilobytes so it should be totally fine to store it in textarea

I have tested that it works for even text size of 256MB
but as it is technically possible for the size to be extremely large (maybe there are lots of exceptions or something)
I implemented a logic that if the file is greater than 1MB, it would writes to tmp/sysinfo.json and then use file= route to retrieve it

for backwards compatibility the existing /internal/sysinfo /internal/sysinfo-download are moved to models.api
the behavior is unchanged except that it now would respect --api-auth and would only be created if --api is enabled

tested to work in Chrome and Firefox


additional change

this line of code triggers a false error in pycharms inspector
I decided to add in because I'm already working on this file

Screenshots/videos:

2024-12-28.22_27_11_567.firefox.mp4

firefox

2024-12-27 22_16_04_744 pycharm64

false error

Checklist:

@w-e-w w-e-w force-pushed the Authenticated-Sysinfo branch from 2e1e35a to c7b3fcb Compare December 27, 2024 02:53
@w-e-w w-e-w marked this pull request as draft December 27, 2024 03:03
@w-e-w w-e-w marked this pull request as ready for review December 27, 2024 03:35
@w-e-w w-e-w marked this pull request as draft December 27, 2024 03:36
@w-e-w w-e-w force-pushed the Authenticated-Sysinfo branch 3 times, most recently from 3525935 to cf8790c Compare December 28, 2024 15:09
while syntactically correct this triggers a false Unresolved reference 'k' error in PyCharms
@w-e-w w-e-w force-pushed the Authenticated-Sysinfo branch from cf8790c to dc34c00 Compare December 28, 2024 15:16
@w-e-w w-e-w marked this pull request as ready for review December 28, 2024 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant